-
Notifications
You must be signed in to change notification settings - Fork 67
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use python executable variable instead of hardcode python3 #239
Conversation
Signed-off-by: Jose Luis Rivero <[email protected]>
src/python/CMakeLists.txt
Outdated
@@ -110,7 +110,7 @@ if (PYTHONLIBS_FOUND) | |||
|
|||
foreach (test ${python_tests}) | |||
add_test(NAME ${test}.py COMMAND | |||
python3 ${CMAKE_SOURCE_DIR}/src/python/${test}.py) | |||
"${Python3_EXECUTABLE}" "${CMAKE_SOURCE_DIR}/src/python/${test}.py") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
which cmake call is providing this variable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
find_package(Python3)
here https://github.com/ignitionrobotics/ign-math/pull/239/files#diff-43c9042647d14a73e4061f77bba19ee368fd059a7e0c6086063f7646e2b2aabbR69. Umm, that call is conditionally scoped, I thought it was used in the root CMakeLists.txt but it is the old find_package(PythonLibs)
. We don't have find_package(Python3)
in Bionic, I changed the code to use old PYTHON_EXECUTABLE
variable.
Signed-off-by: Jose Luis Rivero <[email protected]>
Signed-off-by: Steve Peters <[email protected]>
Codecov Report
@@ Coverage Diff @@
## ign-math6 #239 +/- ##
==========================================
Coverage 99.40% 99.40%
==========================================
Files 66 66
Lines 6185 6185
==========================================
Hits 6148 6148
Misses 37 37 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
works on macOS for me now
This should prepare the software for gazebo-tooling/release-tools#508. Note that CI won't run the python/ruby test on Mac/Windows until #508 is ready.